Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor creature pathfinding, and unify monster pathfinding logic. #70274

Closed
wants to merge 47 commits into from

Conversation

prharvey
Copy link
Contributor

@prharvey prharvey commented Dec 18, 2023

Summary

Infrastructure "Refactor/optimize monster pathfinding."

Purpose of change

Monster pathfinding is the single most expensive thing in high traffic areas. While optimizing this in a previous PR, it quickly became clear that the code path was difficult to work with due to pathfinding logic being spread across multiple different parts of the codebase. Bugs in one area would cover for bugs in others, and due to this it wasn't entirely clear what behavior was originally intended.

Describe the solution

Rewrite creature pathfinding to have all pathfinding logic (i.e. can I move here, how much does it cost to move here, find path between x/y, etc) in a single location, such that no logic needs to be duplicated in order to answer questions regarding pathfinding.

This is done by making the pathfinding implementation generic, and having the pathfinding logic take place at the intersection of two different caches: the pathfinding settings, and the map pathfinding cache.

Pathfinding settings is a per-creature structure that encapsulates pathfinding logic specific to that creature, e.g. if it flies, if it can open doors, what fields it is immune to, etc. Caching this logic beforehand allows for fast rejection of bad tiles using a bitfield, which is important when thousands of tiles are visited.

The map pathfinding cache encapsulates pathfinding logic specific to that tile, e.g. if there is an obstacle there, the cost to move on the tile, the existence of traps/fields, etc. This creates a bitfield for the specific tile that allows for fast rejection of a tile when combined with the pathfinding settings.

Note that in the tested location (TCL) the vast majority of paths end up not being found because monsters are stuck behind glass/bars they cannot bash down, or there is a destination in the next room they can hear, but not move to. This is handled by using a bidirectional A* implementation that can fail fast if either point is in a closed in room, and adding exponential backoff to trying to path while stuck.

Finally, as now the pathfinding logic is unified, it means monsters can act much more intelligently. e.g. flying monsters can actually use the ability to fly when finding paths to a destination. To take advantage of this new ability (and to do integration testing), a new flying variant of mi-go was added: the mi-go shrike. It doesn't spawn anywhere yet however, that will be done in a future PR.

Describe alternatives you've considered

As the vast majority of the CPU usage left is due to paths that cannot be found, it would make sense to preprocess the map into a set of islands so these can be quickly rejected. The problem is that it isn't possible in general to do this as each monster type has different bashing thresholds, door avoidance, field avoidance, etc that could result in the creation of an island for one type of monster but not another. This would require per-monster(type) island generation that could quickly take up a lot of memory and processing power. I think it is worth visiting this as a separate step in a future PR: i.e. detect when a path cannot be found, and detect if the monster is stuck on an island it cannot escape from. This should remove what is left of the pathfinding CPU usage, but would make this PR too complicated.

I also briefly experimented with using a weighted variant of Jump Point Search to further optimize this. While in theory this should result in an order of magnitude speedup, the added complexity wasn't worth it for this PR. It is something to try doing in the future. I would tackle this before trying to tackle islands, as if this really does achieve an order of magnitude speedup it would be more generally useful and less error prone.

In the end though, this was largely solved by just using exponential backoff. Sometimes simple is best.

Caching the pathfinding settings is also an option that wasn't done. I'm not comfortable with properly handling the invalidation of that in the same PR. It would be too prone to breaking in weird ways, and didn't want the added complexity.

Testing

Added extensive unit and integration testing for the new pathfinding logic. The old monster movement behavior wasn't tested at all, so I did my best to look at how monsters already behave with the current pathfinding and try to capture that in the tests.

Additional context

Before:
pf_before

After:
new_pf_after

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Map / Mapgen Overmap, Mapgen, Map extras, Map display [C++] Changes (can be) made in C++. Previously named `Code` Monsters Monsters both friendly and unfriendly. Mechanics: Enchantments / Spells Enchantments and spells Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 18, 2023
@Inglonias
Copy link
Contributor

Ooo. This looks super neat.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 18, 2023
@github-actions github-actions bot added the Code: Tests Measurement, self-control, statistics, balancing. label Dec 29, 2023
@prharvey prharvey force-pushed the pathfinding_refactor branch from e1a006a to 083022d Compare December 29, 2023 09:51
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 29, 2023
@github-actions github-actions bot added BasicBuildPassed This PR builds correctly, label assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jan 1, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 1, 2024
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 1, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 2, 2024
@prharvey prharvey changed the title [WIP] Refactor creature pathfinding, and unify monster pathfinding logic. Refactor creature pathfinding, and unify monster pathfinding logic. Jan 2, 2024
@prharvey prharvey marked this pull request as ready for review January 2, 2024 18:10
@github-actions github-actions bot requested a review from KorGgenT January 2, 2024 18:10
@prharvey prharvey marked this pull request as draft January 4, 2024 04:51
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not bump or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@github-actions github-actions bot added the stale Closed for lack of activity, but still valid. label Feb 13, 2024
@github-actions github-actions bot closed this Mar 14, 2024
CLIDragon added a commit to CLIDragon/Cataclysm-DDA that referenced this pull request Aug 25, 2024
See pull request for more details.
@CLIDragon CLIDragon mentioned this pull request Aug 25, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Code: Tests Measurement, self-control, statistics, balancing. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Mechanics: Enchantments / Spells Enchantments and spells Monsters Monsters both friendly and unfriendly. stale Closed for lack of activity, but still valid.
Projects
Status: Abandoned PRs with no one to pick them up
Development

Successfully merging this pull request may close these issues.

3 participants